-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix!: rename ark_project into arkproject and wallet_kit into walletkit #443
fix!: rename ark_project into arkproject and wallet_kit into walletkit #443
Conversation
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~443 Documentation is deployed and generated using docs.page. |
WalkthroughThe pull request refactors various parts of the codebase by standardizing naming conventions. The dependency and directory references for the wallet kit have been changed from Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as CreateNewWalletButton
participant WS as WalletService
participant C as CreateWalletScreen
U->>B: Tap "Create Wallet" button
B->>WS: Call newSeedPhrase()
WS-->>B: Return new seed phrase
B->>C: Navigate to CreateWalletScreen with seed phrase
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/wallet_kit/lib/wallet_screens/create_wallet_screen.dart (1)
125-126
: Remove the ignore comment and handle the async context properly.The
use_build_context_synchronously
lint ignore comment suggests a potential issue with usingBuildContext
after an asynchronous gap.Apply this diff to handle the async context properly:
- // ignore: use_build_context_synchronously - ScaffoldMessenger.of(context).showSnackBar( + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
docs/examples/mobile-wallet.mdx
(5 hunks)docs/how-to-contribute.mdx
(2 hunks)examples/nft_marketplace/lib/main.dart
(1 hunks)examples/nft_marketplace/lib/screens/home_screen.dart
(1 hunks)examples/nft_marketplace/pubspec.yaml
(1 hunks)examples/wallet_app/lib/main.dart
(1 hunks)examples/wallet_app/lib/screens/home_screen.dart
(1 hunks)examples/wallet_app/pubspec.yaml
(1 hunks)packages/ark_project/example/ark_example.dart
(1 hunks)packages/ark_project/pubspec.yaml
(1 hunks)packages/wallet_kit/ios/Runner/GeneratedPluginRegistrant.m
(2 hunks)packages/wallet_kit/lib/services/wallet_service.dart
(1 hunks)packages/wallet_kit/lib/ui/button.dart
(1 hunks)packages/wallet_kit/lib/ui/input.dart
(1 hunks)packages/wallet_kit/lib/ui/modal.dart
(1 hunks)packages/wallet_kit/lib/wallet_kit.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/add_wallet_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/create_wallet_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/password_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/protect_wallet_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/recover_wallet_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_screens/settings_screen.dart
(1 hunks)packages/wallet_kit/lib/wallet_state/wallet_provider.dart
(1 hunks)packages/wallet_kit/lib/wallet_state/wallet_state.dart
(1 hunks)packages/wallet_kit/lib/widgets/account_address.dart
(1 hunks)packages/wallet_kit/lib/widgets/icon.dart
(2 hunks)packages/wallet_kit/lib/widgets/nft_details.dart
(1 hunks)packages/wallet_kit/lib/widgets/nft_list.dart
(1 hunks)packages/wallet_kit/lib/widgets/send_eth_button.dart
(1 hunks)packages/wallet_kit/lib/widgets/token_icon.dart
(2 hunks)packages/wallet_kit/lib/widgets/token_list.dart
(1 hunks)packages/wallet_kit/lib/widgets/wallet_body.dart
(1 hunks)packages/wallet_kit/lib/widgets/wallet_list.dart
(1 hunks)packages/wallet_kit/lib/widgets/wallet_selector.dart
(1 hunks)packages/wallet_kit/macos/Flutter/GeneratedPluginRegistrant.swift
(1 hunks)packages/wallet_kit/pubspec.yaml
(2 hunks)packages/wallet_kit/test/utils/format_address_test.dart
(1 hunks)packages/wallet_kit/test/utils/group_by_test.dart
(1 hunks)
✅ Files skipped from review due to trivial changes (22)
- packages/ark_project/pubspec.yaml
- packages/wallet_kit/lib/wallet_state/wallet_provider.dart
- packages/wallet_kit/lib/ui/input.dart
- examples/nft_marketplace/lib/screens/home_screen.dart
- packages/wallet_kit/lib/services/wallet_service.dart
- packages/ark_project/example/ark_example.dart
- packages/wallet_kit/test/utils/group_by_test.dart
- packages/wallet_kit/lib/widgets/account_address.dart
- examples/nft_marketplace/lib/main.dart
- packages/wallet_kit/pubspec.yaml
- packages/wallet_kit/lib/widgets/wallet_selector.dart
- packages/wallet_kit/lib/widgets/nft_list.dart
- packages/wallet_kit/lib/wallet_state/wallet_state.dart
- examples/wallet_app/lib/screens/home_screen.dart
- packages/wallet_kit/test/utils/format_address_test.dart
- packages/wallet_kit/lib/wallet_screens/settings_screen.dart
- packages/wallet_kit/lib/widgets/wallet_list.dart
- packages/wallet_kit/lib/wallet_screens/recover_wallet_screen.dart
- packages/wallet_kit/lib/ui/button.dart
- packages/wallet_kit/lib/wallet_screens/protect_wallet_screen.dart
- examples/wallet_app/lib/main.dart
- packages/wallet_kit/lib/widgets/nft_details.dart
🔇 Additional comments (23)
packages/wallet_kit/lib/widgets/token_icon.dart (1)
3-4
: LGTM! Import and asset path changes align with the package renaming.The changes correctly implement:
- Switching to relative imports for internal modules
- Updating the asset path to use the new package name
Also applies to: 16-16
packages/wallet_kit/lib/widgets/icon.dart (1)
3-4
: LGTM! Import and asset path changes align with the package renaming.The changes correctly implement:
- Switching to relative imports for internal modules
- Updating the asset path to use the new package name
Also applies to: 27-27
packages/wallet_kit/lib/widgets/send_eth_button.dart (1)
5-9
: LGTM! Import changes align with the package renaming.The changes correctly implement:
- Switching to relative imports for internal modules
- Well-organized imports by category
packages/wallet_kit/lib/wallet_kit.dart (1)
6-7
: LGTM! Import changes align with the package renaming.The changes correctly implement switching to relative imports for internal modules.
packages/wallet_kit/lib/wallet_screens/add_wallet_screen.dart (2)
3-7
: LGTM! Import changes align with package renaming.The switch from absolute to relative imports is consistent with the PR objective of renaming the package.
39-41
: Verify seed phrase generation.The code now uses
WalletService.newSeedPhrase()
for generating seed phrases. Please ensure this method follows cryptographic best practices.✅ Verification successful
Seed Phrase Generation Implementation Verified
- The
newSeedPhrase()
method directly leveragesbip39.generateMnemonic()
, a widely adopted implementation of the BIP39 specification for mnemonic seed phrases.- Provided the
bip39
package is up-to-date and correctly implemented, this approach adheres to cryptographic best practices.- No issues were detected in the seed phrase generation mechanism.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check seed phrase generation implementation ast-grep --pattern $'class WalletService { $$$ newSeedPhrase() { $$$ } $$$ }'Length of output: 90
Script:
#!/bin/bash # Search for the definition of WalletService and its newSeedPhrase method in all Dart files echo "Searching for WalletService definition..." rg -n "class\s+WalletService" --glob "*.dart" echo "" echo "Searching for newSeedPhrase method..." rg -n "newSeedPhrase\s*\(" --glob "*.dart"Length of output: 694
Script:
#!/bin/bash # Extract the implementation of the newSeedPhrase method from WalletService sed -n '/static String newSeedPhrase/,/}/p' packages/wallet_kit/lib/services/wallet_service.dartLength of output: 173
packages/wallet_kit/lib/widgets/token_list.dart (1)
4-7
: LGTM! Import changes align with package renaming.The switch from absolute to relative imports is consistent with the PR objective of renaming the package.
packages/wallet_kit/lib/widgets/wallet_body.dart (1)
4-8
: LGTM! Import changes align with package renaming.The switch from absolute to relative imports is consistent with the PR objective of renaming the package.
packages/wallet_kit/lib/wallet_screens/create_wallet_screen.dart (1)
5-7
: LGTM! Import changes align with package renaming.The switch from absolute to relative imports is consistent with the PR objective of renaming the package.
packages/wallet_kit/lib/wallet_screens/password_screen.dart (2)
5-7
: LGTM! Import statements updated correctly.The change from absolute to relative imports aligns with the package renaming and improves modularity.
119-120
: Good enhancement to the PasswordScreen component.The addition of an optional callback parameter with a sensible default improves the component's reusability.
Also applies to: 126-127
packages/wallet_kit/lib/ui/modal.dart (2)
2-3
: LGTM! Import statements simplified.The removal of the wallet_kit import and addition of a direct button import aligns with the package renaming effort.
82-126
: Good cleanup of commented code.Removing the commented-out StatefulWidget implementation improves code maintainability.
examples/wallet_app/pubspec.yaml (1)
12-12
: LGTM! Package dependency renamed correctly.The dependency has been updated from
wallet_kit
towalletkit
as intended.examples/nft_marketplace/pubspec.yaml (1)
31-31
: LGTM! Package dependencies renamed correctly.Both dependencies have been updated as intended:
ark_project
→arkproject
wallet_kit
→walletkit
Also applies to: 35-35
docs/how-to-contribute.mdx (2)
100-100
: Ensure Consistent Package Renaming for Walletkit Documentation.
The change fromwallet_kit
towalletkit
in the repository structure is clearly reflected here. Please double-check that all related documentation references throughout the project have been updated accordingly.
122-122
: Confirm the Starknet Devnet Version Downgrade.
The documentation now states Starknet Devnet version as0.1.2
. Given this is a significant downgrade from previously mentioned versions, please verify that this change is intentional and that no compatibility issues exist with the current setup instructions.docs/examples/mobile-wallet.mdx (5)
20-20
: Update Dependency Command with Renamed Package.
The dependency addition command now useswalletkit
instead ofwallet_kit
, which aligns with the PR objective. Ensure that users update theirpubspec.yaml
accordingly if they copy this command for their projects.
47-47
: Clarify Android minSdk Requirement Note.
The note now correctly mentions that thesecure_store
package (used bywalletkit
) requires a minimum SDK of 23. This explicit instruction is helpful—just confirm that all platforms are covered by similar documentation if needed.
102-102
: Verify Updated Import Statement in Main Function Block.
The import statement now readsimport 'package:walletkit/wallet_kit.dart';
, reflecting the package renaming. Please ensure that the corresponding file name and package configuration remain consistent with this change.
147-147
: Ensure Consistency in Import Usage Across Code Blocks.
The updated import in the second main code block also useswalletkit
, which is consistent. It might be worth checking that the file name being imported (wallet_kit.dart
) aligns with the new package naming conventions—or if further renaming is required.
198-198
: Document Pre-Built Layout Integration from Walletkit.
The instructions now clearly state to create ascreens/
folder and add ahome_screen.dart
file using pre-built layouts fromwalletkit
(including components likeWalletSelector
,AccountAddress
, etc.). This guidance is useful; please verify that all required components are correctly exported from thewalletkit
package.packages/wallet_kit/ios/Runner/GeneratedPluginRegistrant.m (1)
27-28
: Verify WebView Flutter plugin header path and registration.The changes to both the import statement and plugin registration are consistent with the macOS implementation. Let's verify the header file path and plugin registration.
Also applies to: 39-39
✅ Verification successful
WebView Flutter plugin registration on iOS is correct.
The iOS GeneratedPluginRegistrant file now correctly imports the header using
<webview_flutter_wkwebview/WebViewFlutterPlugin.h>
, and there are no outdated references in the iOS context. The output from our searches confirms that only macOS-generated files still reference “FLTWebViewFlutterPlugin,” which is expected for that platform.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the WebView Flutter plugin header path and registration # Check if the header file exists at the specified path fd WebViewFlutterPlugin.h # Check for any remaining references to the old plugin name rg "FLTWebViewFlutterPlugin" --type swift --type objcLength of output: 416
@@ -12,5 +12,5 @@ import webview_flutter_wkwebview | |||
func RegisterGeneratedPlugins(registry: FlutterPluginRegistry) { | |||
PathProviderPlugin.register(with: registry.registrar(forPlugin: "PathProviderPlugin")) | |||
SecureStorePlugin.register(with: registry.registrar(forPlugin: "SecureStorePlugin")) | |||
FLTWebViewFlutterPlugin.register(with: registry.registrar(forPlugin: "FLTWebViewFlutterPlugin")) | |||
WebViewFlutterPlugin.register(with: registry.registrar(forPlugin: "WebViewFlutterPlugin")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Inconsistent WebView Plugin Naming Detected
- The main file in
packages/wallet_kit/macos/Flutter/GeneratedPluginRegistrant.swift
registers the plugin asWebViewFlutterPlugin
, which is in line with the intended standardization. - However, the search shows that example projects (in
examples/nft_marketplace
andexamples/wallet_app
) still referenceFLTWebViewFlutterPlugin
.
It’s recommended to update the example projects to ensure consistency across the repository.
🔗 Analysis chain
Verify WebView Flutter plugin naming consistency.
The change from FLTWebViewFlutterPlugin
to WebViewFlutterPlugin
appears correct, but let's verify this is consistent with the plugin's actual naming in the Flutter ecosystem.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the correct plugin name for webview_flutter_wkwebview
# Check the plugin's pubspec.yaml for the correct plugin class name
rg -A 5 "plugin:" packages/wallet_kit/pubspec.yaml
# Check for any remaining references to the old plugin name
rg "FLTWebViewFlutterPlugin" --type swift --type objc
Length of output: 439
Due to publishing issue:
ark_project
package intoarkproject
wallet_kit
package intowalletkit
Summary by CodeRabbit
Documentation
Refactor
Chores